primops: add builtins.convertHash#7708
Conversation
|
This should probably be "base" should probably be "encoding", since "sri" is not a base. The |
|
Thanks for reviewing!
Regarding this, we could define This could be done as a builtin (using either BTW, we would need to support colon-seperated hash prefix also, since |
3cf1f49 to
e29457c
Compare
|
Just rename to |
|
You're my hero. |
e29457c to
b81b954
Compare
|
|
Do you guys think I should rename |
8324e57 to
3114296
Compare
I think this should be changed to a single function where you can specify the output format, and perhaps optionally the input format. It should take an attribute set instead of positional arguments, to make clear which string is what. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2023-06-09-nix-team-meeting-minutes-61/29163/1 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
Sorry for late reply. I need some time to mourn for the catastrophic data loss on my laptop (~300 GiB disappeared after reboot) and restore the environment.
What is the proper way to read in an attribute set for a built-in function? There seems to be a variety of implementation inside |
fricklerhandwerk
left a comment
There was a problem hiding this comment.
Hey @ShamrockLee, I'll be available for getting this merged. I suggest to start with examples and usage notes for the docs, so we're sure the interface is smooth. Implementation is already largely there and should be straightforward.
|
@ShamrockLee do you still have time to pursue this further? I'm really sorry that PRs here have such long turnaround times. We're working on improving that, step by step. |
|
Another force push to clean up temporary files accidentally committed or remainings across rebase. |
fricklerhandwerk
left a comment
There was a problem hiding this comment.
Ran the tests and played with it, it works. A few superficial items on documentation/presentation.
c7a0009 to
84f150a
Compare
84f150a to
2d37f33
Compare
| /** | ||
| * Parse a string representing a hash format. | ||
| */ | ||
| HashFormat parseHashFormat(std::string_view hashFormatName); | ||
|
|
||
| /** | ||
| * std::optional version of parseHashFormat that doesn't throw error. | ||
| */ | ||
| std::optional<HashFormat> parseHashFormatOpt(std::string_view hashFormatName); | ||
|
|
||
| /** | ||
| * The reverse of parseHashFormat. | ||
| */ | ||
| std::string_view printHashFormat(HashFormat hashFormat); | ||
|
|
There was a problem hiding this comment.
These should have unit tests that they round-trip. Ideally also a rapidcheck property test too.
There was a problem hiding this comment.
Round-trip test added.
I'm not familiar with rapidcheck property check. Are there examples of such tests? I'm still trying to figure out what https://github.com/emil-e/rapidcheck/blob/master/doc/properties.md would like to explain.
There was a problem hiding this comment.
There are examples of such tests, look at the store path tests.
4c21b46 to
947e60b
Compare
Remove redundant "else" after "return". Use std::nullopt to increase readability.
List the allowed hash formats
hashBase is ambiguous, since it's not about the digital bases, but about the format of hashes. Base16, Base32 and Base64 are all character maps for binary encoding. Rename the enum Base to HashFormat. Rename variables of type HashFormat from [hash]Base to hashFormat, including CmdHashBase::hashFormat and CmdToBase::hashFormat.
Base* -> HashFormat::Base*
Add hash format analogy of parseHashTypeOpt, parseHashType, and printHashType. Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
947e60b to
5088e65
Compare
|
(Rebase to resolve merge conflict.) |
|
@ShamrockLee thank you very much for pulling through with this! |
Motivation
This PR adds a builtin function
builtins.convertHashthat converts a hash string to another format.Context
Solves #3151
Cc: @nlewo
Examples:
Type:
To implement
builtins.convertHash, the hash algorithm name (if specified) is parsed withparseHashType, and the hash format name withparseHashFormat. The original hash and the algorithm then feed toHash::parseAny, and produce the result through its methodto_stringwith the parsed hash format taken.The attribute names are inspired by the
outputHash, andoutputHashAlgoused to specify FOD, andnix-hash --to-base*|--to-sriused to convert hash formats in the command line.This PR also renames tree-wide from "hash base" to "hash format" to avoid confusion. This includes:
enum Basetoenum HashFormat.CmdHashFormat::basetoCmdHashFormat::hashFormatCmdToBase::basetoCmdToBase::hashFormathashBaseorbasetohashFormat.Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/teststests/nixos/*